Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

heed NO_COLOR environment variable #8693

Merged
merged 1 commit into from
Feb 6, 2023
Merged

heed NO_COLOR environment variable #8693

merged 1 commit into from
Feb 6, 2023

Conversation

moneromooo-monero
Copy link
Collaborator

No description provided.

@moneromooo-monero moneromooo-monero mentioned this pull request Jan 1, 2023
Closed
SChernykh
SChernykh reviewed Jan 1, 2023
View reviewed changes
contrib/epee/src/mlog.cpp
static bool is_nocolor()
{
static const char *no_color_var = getenv("NO_COLOR");
static const bool no_color = no_color_var && *no_color_var; // apparently, NO_COLOR=0 means no color too (as per no-color.org)
Copy link
Contributor

@SChernykh SChernykh Jan 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use strtol to check the actual numeric value if you want to compare with 0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to compare with 0, 0 apparently means the same as 1 or yes or no (according to the page). The contents do not matter, only their existence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've just read the docs too and it explicitly say "any non-empty string", so it's better to leave the code as is.

jeffro256
jeffro256 approved these changes Jan 2, 2023
View reviewed changes
Copy link
Contributor

@jeffro256 jeffro256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me! The only xterm control sequences produced with this PR + the env variable are bracketed paste sequences.

@moneromooo-monero
Copy link
Collaborator Author

moneromooo-monero commented Jan 2, 2023 •
edited
Loading

I had no idea what bracketed paste sequences were. Having looked that up, I have no idea what could be generating those, wikipedia says it's a tty thing. Where do you see those ?

@jeffro256
Copy link
Contributor

jeffro256 commented Jan 2, 2023 •
edited
Loading

I tried looking for what could be generating those sequences as well without luck. The sequence is \e[?2004h, and they seem to be randomly dispersed throughout the output, as you can see on this example monerod stdout before the line "Starting p2p net loop".

2023-01-02 19:00:41.114 I Monero 'Fluorine Fermi' (v0.18.1.0-bfd7c00ce)
2023-01-02 19:00:41.114 I Initializing cryptonote protocol...
2023-01-02 19:00:41.114 I Cryptonote protocol initialized OK
2023-01-02 19:00:41.118 I Initializing core...
2023-01-02 19:00:41.118 I Loading blockchain from folder /home/jeff/.bitmonero/lmdb ...
2023-01-02 19:00:43.386 I Loading checkpoints
2023-01-02 19:00:43.387 I Core initialized OK
2023-01-02 19:00:43.387 I Initializing p2p server...
2023-01-02 19:00:43.397 I p2p server initialized OK
2023-01-02 19:00:43.397 I Initializing core RPC server...
2023-01-02 19:00:43.399 I Binding on 127.0.0.1 (IPv4):18081
2023-01-02 19:00:43.403 I core RPC server initialized OK on port: 18081
2023-01-02 19:00:43.407 I Starting core RPC server...
2023-01-02 19:00:43.408 I core RPC server started ok
ESC[?2004h2023-01-02 19:00:43.408       I Starting p2p net loop...
2023-01-02 19:00:44.409 I 
2023-01-02 19:00:44.409 I **********************************************************************
2023-01-02 19:00:44.409 I The daemon will start synchronizing with the network. This may take a long time to complete.
2023-01-02 19:00:44.409 I 
2023-01-02 19:00:44.409 I You can set the level of process detailization through "set_log <level|categories>" command,
2023-01-02 19:00:44.409 I where <level> is between 0 (no details) and 4 (very verbose), or custom category based levels (eg, *:WARNING).
2023-01-02 19:00:44.409 I 
2023-01-02 19:00:44.409 I Use the "help" command to see the list of available commands.
2023-01-02 19:00:44.409 I Use "help <command>" to see a command's documentation.
2023-01-02 19:00:44.409 I **********************************************************************
2023-01-02 19:00:44.418 W Unable to send transaction(s), no available connections
2023-01-02 19:00:47.996 I Version 0.18.1.2 of monero for source is available: https://downloads.getmonero.org/source/monero-source-v0.18.1.2.tar.bz2, SHA256 hash 0b667acba2223f8f082b388d7ceb5d212e0515b8a94395f3ec69183ef4bebf6c
2023-01-02 19:00:50.187 I [142.132.210.169:18080 OUT] Sync data returned a new top block candidate: 2791013 -> 2791030 [Your node is 17 blocks (34.0 minutes) behind] 
2023-01-02 19:00:50.188 I SYNCHRONIZATION started
2023-01-02 19:01:04.815 I Synced 2791030/2791030
2023-01-02 19:01:04.816 I 
2023-01-02 19:01:04.816 I **********************************************************************
2023-01-02 19:01:04.816 I You are now synchronized with the network. You may now start monero-wallet-cli.
2023-01-02 19:01:04.816 I 
2023-01-02 19:01:04.816 I Use the "help" command to see the list of available commands.
2023-01-02 19:01:04.816 I **********************************************************************

Linux is 5.15.0-56-generic
OS is Ubuntu 22.04.1 LTS

Maybe it's a libreadline thing?

@jeffro256
Copy link
Contributor

Maybe related: pexpect/pexpect#669

@jeffro256 jeffro256 mentioned this pull request Jan 2, 2023
Merged
@luigi1111 luigi1111 merged commit c12bc62 into monero-project:master Feb 6, 2023
@jeffro256
Copy link
Contributor

Could we please get this opened against release?

@SChernykh SChernykh mentioned this pull request Oct 31, 2023
Merged
@SChernykh
Copy link
Contributor

@jeffro256 #9047

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SChernykh SChernykh SChernykh approved these changes

@jeffro256 jeffro256 jeffro256 approved these changes

@vtnerd vtnerd vtnerd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@moneromooo-monero @jeffro256 @SChernykh @vtnerd @luigi1111